Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgraded Hook API (2) #1780

Merged
merged 14 commits into from
May 5, 2021
Merged

Upgraded Hook API (2) #1780

merged 14 commits into from
May 5, 2021

Conversation

ranile
Copy link
Member

@ranile ranile commented Mar 3, 2021

Description

This PR takes over the work done by @jkelleyrtp in #1645 as that PR has been inactive for quite a while. I have also applied the review suggestions and updated documentation.

I tried to keep @jkelleyrtp's original commits as much as I could. Almost all of the code updates in this PR are theirs.

Fixes #1644
Fixes #1757
Closes #1645 (supersedes it)

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests

@github-actions
Copy link

github-actions bot commented Mar 3, 2021

Visit the preview URL for this PR (updated for commit a7e23ab):

https://yew-rs--pr1780-hook-api-b96cfhqi.web.app

(expires Wed, 12 May 2021 20:04:04 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@pickfire
Copy link

What about use_state(0) and with_state(|| 0) which use_state uses just the value, but I wonder if we can do some optimization here so that use_state could be faster? Which is similar to rust or(0) and or_else(|| 0). Of course, the naming can be bikeshed.

@ranile
Copy link
Member Author

ranile commented Apr 24, 2021

@pickfire, what's wrong with passing a closure though? It is possible to have a different function but I don't think that offers any benefits.

@jkelleyrtp
Copy link
Contributor

What about use_state(0) and with_state(|| 0) which use_state uses just the value, but I wonder if we can do some optimization here so that use_state could be faster? Which is similar to rust or(0) and or_else(|| 0). Of course, the naming can be bikeshed.

You don't want to pass values directly because they will get stack allocated each time the function is run. The closure prevents the creation of the object when it won't be used - as it probably won't because this is a hook setup. Clippy will warn about this too and discourages the use .or(0) in favor of .or_else(|| 0)

ranile added 2 commits May 3, 2021 22:39
# Conflicts:
#	packages/yew-functional/src/hooks/mod.rs
#	packages/yew-functional/src/lib.rs
Copy link
Member

@siku2 siku2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly some adjustments to use Yew conventions

packages/yew-functional/tests/use_effect.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_state.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_state.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_effect.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_effect.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_effect.rs Outdated Show resolved Hide resolved
packages/yew-functional/tests/use_effect.rs Outdated Show resolved Hide resolved
packages/yew-functional/src/hooks/use_reducer.rs Outdated Show resolved Hide resolved
@siku2 siku2 merged commit aa828d7 into yewstack:master May 5, 2021
@voidpumpkin voidpumpkin added the A-yew Area: The main yew crate label Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More ergonomic use_state hook Better API for hooks
6 participants